upgrade/systemvm: add template zone entries by shwstppr · Pull Request #5642 · apache/cloudstack

shwstppr · 2021-10-29T14:19:00Z

Description

When registering a system VM template during an upgrade, entries in cloud.template_zone_ref must be created for the new template.
For a cross-zones template, an entry for each zone must be added.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Fixes apache#5641 When registering a system VM template during an upgrade, entries in cloud.template_zone_ref must be created for the new template. For a cross-zones template, entry for each zone must be added. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr · 2021-10-29T14:20:16Z

@blueorangutan package

shwstppr · 2021-10-29T14:22:20Z

@blueorangutan package

blueorangutan · 2021-10-29T14:23:03Z

@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-10-29T15:03:39Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1635

nvazquez · 2021-10-29T15:47:19Z

@blueorangutan test matrix

yadvr · 2021-10-29T16:52:30Z

Manual upgrade testing required from 4.14, 4.15 to this PR's pkgs cc @nvazquez @borisstoyanov @vladimirpetrov @sureshanaparti

yadvr · 2021-10-29T16:52:36Z

@blueorangutan test matrix

blueorangutan · 2021-10-29T16:53:03Z

@rhtyd a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

blueorangutan · 2021-10-30T01:42:31Z

Trillian test result (tid-2492)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 30331 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5642-t2492-kvm-centos7.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2021-10-30T02:48:31Z

Trillian test result (tid-2491)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34411 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5642-t2491-xenserver-71.zip
Smoke tests completed. 90 look OK, 1 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_cancel_host_maintenace_with_no_migration_jobs	`Error`	1050.19	test_host_maintenance.py

blueorangutan · 2021-10-30T02:58:34Z

Trillian test result (tid-2493)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35134 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5642-t2493-vmware-65u2.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

weizhouapache

code lgtm
did not test it

shwstppr · 2021-11-01T07:36:30Z

There seems to be some issue even after changes, getting an exception on upgrade,

2021-11-01 07:33:35,313 ERROR [c.c.u.SystemVmTemplateRegistration] (main:null) (logid:) Failed to register template for hypervisor: VMware
java.lang.NullPointerException
	at com.cloud.storage.dao.VMTemplateDaoImpl.addTemplateToZone(VMTemplateDaoImpl.java:474)
	at com.cloud.upgrade.SystemVmTemplateRegistration.createCrossZonesTemplateZoneRefEntries(SystemVmTemplateRegistration.java:468)
	at com.cloud.upgrade.SystemVmTemplateRegistration.performTemplateRegistrationOperations(SystemVmTemplateRegistration.java:600)

Looking into it.
Moving PR into draft meanwhile
@nvazquez @weizhouapache @rhtyd

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr · 2021-11-01T09:39:16Z

@blueorangutan package

blueorangutan · 2021-11-01T09:41:03Z

@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-11-01T10:33:49Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1645

shwstppr · 2021-11-01T11:16:31Z

@nvazquez @weizhouapache @rhtyd
Tested 2 upgrades 4.14.1 and 4.15.2. Both worked fine:

Deployed env
Upgraded to this PR packages without registering system VM template prior to upgrade
Verified cloud.template_zone_ref after upgrade for a new entry for new system VM template
Enabled CK and deployed a k8s cluster

@blueorangutan test matrix

shwstppr · 2021-11-01T11:18:14Z

@blueorangutan test matrix

blueorangutan · 2021-11-01T11:19:03Z

@shwstppr a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

sureshanaparti · 2021-11-01T11:57:42Z

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

+            templateZoneVO = new VMTemplateZoneVO(zoneId, templateId, new java.util.Date());
+            templateZoneVO = vmTemplateZoneDao.persist(templateZoneVO);
+        } else {
+            templateZoneVO.setRemoved(GenericDaoBase.DATE_TO_NULL);


I think, no need to explicitly set removed to null (the record retrieved using findByZoneTemplate() already has removed null)

sureshanaparti · 2021-11-01T12:00:52Z

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

+                templateZoneVO = null;
+            }
+        }
+        return templateZoneVO;


this returned templateZoneVO is unused, and only being validated for null. I think, it is better through the exception when the update fails.

Don't think it affects functionality

Don't think it affects functionality

correct, doesn't affect functionality. it's about the implementation. returning VMTemplateZoneVO is not required in this method.

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

blueorangutan · 2021-11-01T20:21:39Z

Trillian test result (tid-2499)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31059 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5642-t2499-kvm-centos7.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2021-11-01T21:19:55Z

Trillian test result (tid-2500)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34855 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5642-t2500-vmware-65u2.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2021-11-01T21:32:13Z

Trillian test result (tid-2498)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35412 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5642-t2498-xenserver-71.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

vladimirpetrov

LGTM based on manual testing, tested upgrades from:

4.14.1 KVM CentOS 7, 2 zones
4.14.1 VMWare 6.7, 2 zones
4.14.1 XCP-NG 8.2, 1 zone
4.15.2 KVM CentOS 7, 1 zone
4.15.2 VMWare 6.7, 2 zones
4.15.2 XCP-NG 8.2, 2 zones

nvazquez added this to the 4.16.0.0 milestone Oct 29, 2021

nvazquez added Severity:BLOCKER type:bug labels Oct 29, 2021

nvazquez approved these changes Oct 29, 2021

View reviewed changes

nvazquez requested review from Pearl1594, borisstoyanov, davidjumani, vladimirpetrov and weizhouapache October 29, 2021 14:56

nvazquez assigned shwstppr Oct 29, 2021

yadvr approved these changes Oct 29, 2021

View reviewed changes

nvazquez added the status:needs-testing label Oct 29, 2021

weizhouapache approved these changes Nov 1, 2021

View reviewed changes

shwstppr marked this pull request as draft November 1, 2021 07:36

fix for template-zone entry create

960f54b

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr requested review from nvazquez, weizhouapache and yadvr November 1, 2021 11:16

shwstppr marked this pull request as ready for review November 1, 2021 11:17

sureshanaparti reviewed Nov 1, 2021

View reviewed changes

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java Show resolved Hide resolved

sureshanaparti reviewed Nov 1, 2021

View reviewed changes

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java Show resolved Hide resolved

sureshanaparti reviewed Nov 1, 2021

View reviewed changes

change

f193daf

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

yadvr approved these changes Nov 2, 2021

View reviewed changes

vladimirpetrov approved these changes Nov 2, 2021

View reviewed changes

yadvr merged commit d717d8a into apache:main Nov 2, 2021

weizhouapache added a commit to shapeblue/cloudstack that referenced this pull request Nov 2, 2021

engine/schema: fix build error in apache#5642

bf3c4ab

weizhouapache mentioned this pull request Nov 2, 2021

engine/schema: fix build error in #5642 #5657

Merged

12 tasks

weizhouapache added a commit that referenced this pull request Nov 2, 2021

engine/schema: fix build error in #5642 (#5657)

1928723

DaanHoogland removed the status:needs-testing label Jan 28, 2026

Conversation

shwstppr commented Oct 29, 2021

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

shwstppr commented Oct 29, 2021

Uh oh!

shwstppr commented Oct 29, 2021

Uh oh!

blueorangutan commented Oct 29, 2021

Uh oh!

blueorangutan commented Oct 29, 2021

Uh oh!

nvazquez commented Oct 29, 2021

Uh oh!

yadvr commented Oct 29, 2021

Uh oh!

yadvr commented Oct 29, 2021

Uh oh!

blueorangutan commented Oct 29, 2021

Uh oh!

blueorangutan commented Oct 30, 2021

Uh oh!

blueorangutan commented Oct 30, 2021

Uh oh!

blueorangutan commented Oct 30, 2021

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

shwstppr commented Nov 1, 2021

Uh oh!

shwstppr commented Nov 1, 2021

Uh oh!

blueorangutan commented Nov 1, 2021

Uh oh!

blueorangutan commented Nov 1, 2021

Uh oh!

shwstppr commented Nov 1, 2021

Uh oh!

shwstppr commented Nov 1, 2021

Uh oh!

blueorangutan commented Nov 1, 2021

Uh oh!

Uh oh!

Uh oh!

sureshanaparti Nov 1, 2021

Choose a reason for hiding this comment

Uh oh!

sureshanaparti Nov 1, 2021

Choose a reason for hiding this comment

Uh oh!

shwstppr Nov 2, 2021

Choose a reason for hiding this comment

Uh oh!

sureshanaparti Nov 2, 2021

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Nov 1, 2021

Uh oh!

blueorangutan commented Nov 1, 2021

Uh oh!

blueorangutan commented Nov 1, 2021

Uh oh!

vladimirpetrov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants